-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change error codes from 16-bit to varints #2680
Conversation
Changes CONNECTION_CLOSE, STOP_SENDING, and RESET_STREAM to varint error codes. Fixes #2672
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two relatively important things here.
draft-ietf-quic-transport.md
Outdated
@@ -5550,7 +5554,7 @@ The initial contents of this registry are tabulated in {{frame-types}}. | |||
IANA \[SHALL add/has added] a registry for "QUIC Transport Error Codes" under a | |||
"QUIC Protocol" heading. | |||
|
|||
The "QUIC Transport Error Codes" registry governs a 16-bit space. This space is | |||
The "QUIC Transport Error Codes" registry governs a 62-bit space. This space is | |||
split into two spaces that are governed by different policies. Values with the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we want to crib from the frame types (or other varint registries) when it comes to policies. There isn't really a "first byte" any more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done.
For consistency you probably want to update HTTP too, e.g. there are some references to 16-bit error code there too https://tools.ietf.org/html/draft-ietf-quic-http-20#section-10.5 |
Thanks for pointing that out @LPardue, http updated. |
Co-Authored-By: Mike Bishop <mbishop@evequefou.be>
Mike's suggestion, carrying -> containing
Thanks Mike, I think I fixed all your suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the registry text. LGTM.
Changes CONNECTION_CLOSE, STOP_SENDING, and RESET_STREAM to varint error codes.
Fixes #2672
Also fixes an error where the spec said the transport parameter registry is 16 bits, but transport parameters are now varints as well.